[Bernard Wan De Yuan] iP by bernardwan · Pull Request #461 · nus-cs2103-AY2122S1/ip

bernardwan · 2021-08-25T18:50:41Z

DUKE

“Your mind is for having ideas, not holding them.” – David Allen (source)

Hi this is my Duke program! It's

Fun 👍
Easy to use
text-based

Steps to use:

Download from here
Run
Use!

Features:

Managing tasks
GUI

Sample code main :

public static void main(String[] args) {
        new Duke().run();
    }

# Conflicts: # src/main/java/Duke.java # src/main/java/Task.java # text-ui-test/EXPECTED.TXT

# Conflicts: # src/main/java/duke/DukeException.java # src/main/java/duke/TaskList.java

david-eom

LGTM! The code looks clean and easy to read, with most of the comments being nit-picky 😂 Think that the logic can be a bit more simplified in Parser, and that some javadoc is missing. Excellent code, keep up the good work!! 👍

david-eom · 2021-09-02T08:32:02Z

+import duke.DukeException;
+import duke.Task;
+import duke.TaskList;
+import org.junit.jupiter.api.Test;
+
+import java.util.ArrayList;


Should java related imports come before duke?

david-eom · 2021-09-02T08:36:08Z

@@ -0,0 +1,42 @@
+package duke.Command;
+
+import duke.*;


I think perhaps can consider importing individual classes instead of using wildcard import!

david-eom · 2021-09-02T08:36:27Z

+        if(type.equals("todo")) {
+            task = new Task.Todo(description, false);
+
+        } else if(type.equals("deadline")) {
+            String[] temp = description.split("by ", 2);
+            task = new Task.Deadline(temp[0], false, temp[1]);
+
+        } else if(type.equals("event")) {
+            String[] temp = description.split("at ", 2);
+            task = new Task.Event(temp[0], false, temp[1]);


Maybe can consider using enum for this!

david-eom · 2021-09-02T08:41:02Z

+     * Loads task data from storage
+     *
+     * @return ArrayList of tasks
+     * @throws DukeException


Feel like the condition when the DukeException is thrown should be specified here :))

david-eom · 2021-09-02T08:41:43Z

+        this.status= status;
+    }
+
+    public static class Todo extends Task{


Should there be a whitespace before the curly braces?

david-eom · 2021-09-02T08:49:48Z

+        if (input.equals("bye")) {
+            return new ExitCommand();
+        } else if (input.equals("list")) {
+            return new ListCommand();
+        } else if (first_word.equals("done")) {
+            int index;
+            try {
+                index = Integer.parseInt(input.split(" ")[1]) - 1;
+            } catch (ArrayIndexOutOfBoundsException e) {
+                throw new DukeException("Sorry, please enter an integer after 'done'. (e.g. done 2)");
+            }
+            return new DoneCommand(index);
+        } else if (first_word.equals("delete")) {
+            int index;
+            try {
+                index = Integer.parseInt(input.split(" ")[1]) - 1;
+            } catch (ArrayIndexOutOfBoundsException e) {
+                throw new DukeException("Sorry, please enter an integer after 'delete'. (e.g. delete 2)");
+            }
+            return new DeleteCommand(index);
+        } else if (first_word.equals("find")){
+            String remaining;
+            try {
+                remaining = input.split(" ", 2)[1];
+            } catch (ArrayIndexOutOfBoundsException e) {
+                throw new DukeException("Sorry, please enter a keyword after 'find'.");
+            }
+            return new FindCommand(remaining);
+        } else {
+            String remaining;
+            try {
+                remaining = input.split(" ", 2)[1];
+            } catch (ArrayIndexOutOfBoundsException e) {
+                throw new DukeException("Sorry, tasks must include descriptions.");
+            }
+            return new AddCommand(first_word, remaining);
+        }


Perhaps we can use enum here to simplify the logic here! Also, personally I fell that all the try-catch statement here can be abstracted to other class method e.g. createDone() etc. to prevent arrowhead style code, and that one single catch for ArrayIndexOutOfBoundsException could be used for the entire method. Don't know if I articulate my idea in a very accurate way 😂

david-eom · 2021-09-02T08:50:11Z

+            String line = reader.nextLine();
+            String[] data = line.split(" \\| ");
+            switch (data[0]) {
+                case "todo":


Should case be on the same indent level as switch?

chengseong

LGTM! I think the code is very clean and easy to read and there is very clear abstraction between classes. Well done!

chengseong · 2021-09-04T04:36:25Z

@@ -0,0 +1,30 @@
+package duke.Command;
+
+import duke.*;


I think you should import package by package, for example import duke.TaskList rather than using import duke.*

chengseong · 2021-09-04T04:39:57Z

+public abstract class Command {
+    public abstract void execute(TaskList tasks, Ui ui, Storage storage) throws DukeException;
+
+    public abstract boolean isExit();


You might want to consider setting isExit to False in the abstract, then only changing it to True when it is an ExitCommand. This way you do not have to constantly check what Command it is

Gradle run and shadowJar tasks are not functioning as intended. This should be fixed for ease of use and to create a new JAR release. Let's edit the build.gradle for proper main class declaration. Also,let's fix any checkstyle errors that might inhibit the build process.

There are some left over code from before GUI was implemented. These leftover code that is not used should be removed to improve code quality and readability. Let's check through the code and remove any unused methods or other files.

branch-A-Assertions

Branch a code quality

damithc and others added 23 commits July 29, 2021 20:30

Add Gradle support

d839859

Level-1

f38a22a

Level-2

b98c16f

Level-3

feac61e

Level-4

83381b8

Added automated text UI testing

3c1011c

Implemented Level-5, updated test cases

7551481

Implemented Level-6. updated tests

2565300

Implemented Level-7

5ff4233

Implemented Level-8

0266c78

Merge branch 'branch-Level-8'

33921a6

# Conflicts: # src/main/java/Duke.java # src/main/java/Task.java # text-ui-test/EXPECTED.TXT

Updated tests

48d97f1

Resolved conflict errors

09721a2

Refactor code to make it more OOP

e4d0342

Organise into package

c3e9b99

Add Junit tests

6db07e9

Create JAR File

fe6833c

Add Javadocs

5f6bf33

Tweak code to follow coding standard

9dbf7dd

Implement Level-9 Find feature

06609b4

Merge branch 'branch-A-CodingStandard'

d093898

Merge branch 'branch-Level-9'

d2d6805

# Conflicts: # src/main/java/duke/DukeException.java # src/main/java/duke/TaskList.java

Fix minor javadocs errors

bcefd1c

david-eom reviewed Sep 2, 2021

View reviewed changes

Merge remote-tracking branch 'origin/add-gradle-support'

679bfa8

chengseong reviewed Sep 4, 2021

View reviewed changes

bernardwan added 4 commits September 5, 2021 01:56

Add Gradle to project

341051c

Add ability to use checkstyle

5dcd832

Add GUI

62293f6

bernardwan force-pushed the master branch from 9befeef to c786acd Compare September 5, 2021 20:06

bernardwan added 12 commits September 10, 2021 05:18

Add Assert statements

9d41498

Check code for code quality and clean up code

e710891

There are some left over code from before GUI was implemented. These leftover code that is not used should be removed to improve code quality and readability. Let's check through the code and remove any unused methods or other files.

Merge pull request #1 from bernardwan/branch-A-Assertions

f787e9d

branch-A-Assertions

Merge branch 'master' into branch-A-CodeQuality

879b551

Merge pull request #2 from bernardwan/branch-A-CodeQuality

558ea60

Branch a code quality

Implement B-DoWithinPeriodTasks

fceed2d

Refactor code

fd12c10

Add User Guide

a57ca3c

Rename boolean variable

d45e077

Improve GUI

6e830cf

Handle some exceptions and change font

4714d9b

Update README.md

0d11fb8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Bernard Wan De Yuan] iP#461

[Bernard Wan De Yuan] iP#461
bernardwan wants to merge 40 commits into
nus-cs2103-AY2122S1:masterfrom
bernardwan:master

bernardwan commented Aug 25, 2021 •

edited

Loading

Uh oh!

david-eom left a comment •

edited

Loading

Uh oh!

david-eom Sep 2, 2021

Uh oh!

david-eom Sep 2, 2021

Uh oh!

david-eom Sep 2, 2021

Uh oh!

david-eom Sep 2, 2021

Uh oh!

david-eom Sep 2, 2021

Uh oh!

david-eom Sep 2, 2021

Uh oh!

david-eom Sep 2, 2021

Uh oh!

chengseong left a comment

Uh oh!

chengseong Sep 4, 2021

Uh oh!

chengseong Sep 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

bernardwan commented Aug 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DUKE

Uh oh!

david-eom left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chengseong left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bernardwan commented Aug 25, 2021 •

edited

Loading

david-eom left a comment •

edited

Loading